Skip to content

Conversation

@Jolanrensen
Copy link
Collaborator

@Jolanrensen Jolanrensen commented Oct 30, 2025

Fixes #659

KeyValueProperty and NameValuePair are both data schema's defining a String and a value, though they are incompatible and are used interchangeably in different parts of the library. The only difference was the name for the key (either key or name) and the fact that one is an interface and the other a data class.

This PR brings them closer together:

  • making NameValuePair inherit KeyValueProperty
  • Deprecating name in favor of key
  • Adding deprecated overloads

At a later stage we can deprecate NameValuePair and introduce an instantiable KeyValueProperty with a better name.

@Allex-Nik NameValuePair is marked @RequiredByIntellijPlugin. Do you think these changes will cause issues?

@Jolanrensen Jolanrensen added the enhancement New feature or request label Oct 30, 2025
@Jolanrensen Jolanrensen force-pushed the NameValuePair-KeyValueProperty branch from 5b7e7e4 to 0c0d96f Compare October 30, 2025 14:03
@Allex-Nik
Copy link
Collaborator

@Allex-Nik NameValuePair is marked @RequiredByIntellijPlugin. Do you think these changes will cause issues?

Yes, it will, but it can be easily fixed.

The type renderer for NameValuePair in the IDEA plugin in its current state accesses the name and value as fields, and displays them in the debugger. These changes prevent the renderer from accessing the name field (apparently, it cannot access the deprecated extension property name this way), and the fields are not displayed in the debugger for a NameValuePair.

However, in the renderer we can just access the key field in case access to name was unsuccessful, and it will work (I checked it with your changes), so it will be compatible with the versions of the library having new implementation of NameValuePair as well as with the previous versions.

We could also try to access the deprecated name property via JDI in the plugin, but it seems to me that checking two fields instead of one would be a better solution :)

@Jolanrensen
Copy link
Collaborator Author

@Allex-Nik Great! thanks! :)

I tried keeping name as a deprecated getter to keep binary compatibility, but @DataSchema still detects it and generates non-deprecated accessors for the field :/ so I have to take it out of the class unfortunately

Copy link
Collaborator

@AndreiKingsley AndreiKingsley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a later stage we can deprecate NameValuePair and introduce an instantiable KeyValueProperty with a better name.

So maybe mention this in KDoc then?

@Jolanrensen Jolanrensen force-pushed the NameValuePair-KeyValueProperty branch from 0c0d96f to a4cae3c Compare November 3, 2025 12:41
@Jolanrensen Jolanrensen merged commit 84e044f into master Nov 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge KeyValueProperty and NameValuePair

5 participants